docs: spec §4.2 never re-points --name entries; document CACHEDIR.TAG and store sharding#50
Merged
Merged
Conversation
… and store sharding
Black-box testing against the built binary surfaced a spec ↔ implementation
contradiction and two documentation gaps:
- §4.2 entry resolution (both design docs) still said an explicit --name plus
repo is "treated as a deliberate re-point" and updates the entry. The
implementation has rejected that with exit 2 since the 2026-06 regression
fix ("add never silently re-points an existing entry to another
repository"), and the testing manuals agree — only the spec lagged. The
spec now matches: same repo → update, different repo → error, re-pointing
(like renaming) is the remove + add path.
- New REQ-ADD-NOREPOINT row locks the behavior into the coverage ratchet,
referenced by the existing TestAdd_repointWithNameRejected.
- §5.4 cache tree now shows CACHEDIR.TAG (backup tools skip it) and §4.7
documents that cache clean requires the marker before deleting, so a
misconfigured GRAFT_CACHE_DIR can never remove user data. The content-store
paragraph spells out the on-disk sharding (bucket = first 2 hash digits,
entry dir = remaining 62), which the <xx>/<hex…> notation left ambiguous.
- testing.md: the pseudo-version scenario now removes the dep first (on an
existing entry, a branch ref resolving to the already-locked commit hits
the "already at" no-op and keeps the tag, so the old sequence could not
reproduce REQ-ADD-PSEUDO); the fixture-repo table no longer pins mint tag
versions that drift upstream; cache dir row mentions CACHEDIR.TAG.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the black-box test run of the built binary (all ~60 scenarios passed; every finding was documentation lag, fixed here).
Spec ↔ implementation contradiction (the real one)
docs/design.zh-TW.md/docs/design.md§4.2 still said thatgraft add <repo> --name <existing>"is treated as a deliberate re-point" and updates the entry'srepo. The implementation has rejected this with exit 2 since the 2026-06 regression fix, the error message saysadd never silently re-points an existing entry to another repository, and both testing manuals assert the rejection — only the spec lagged.Resolution: spec updated to match the implementation (rejection). Rationale, recorded in design.local §10.17: the failure modes are asymmetric — allowing re-points means a pasted-wrong repo URL silently swaps vendored code (worst case for a tool whose pitch is content-hash supply-chain safety), while rejecting costs one explicit
remove+addwith an honest diff. It also unifies the model: once aname↔repobinding exists, any identity change goes through remove + add, same as renaming.REQ-ADD-NOREPOINTrow inrequirements.mdlocks it into the coverage ratchet; the existingTestAdd_repointWithNameRejectedcarries the// spec:annotation (comment-only change).CACHEDIR.TAG (previously undocumented)
The cache root contains a
CACHEDIR.TAGmarker: backup tools skip it, andgraft cache cleanrefuses to delete a directory without it — so a misconfiguredGRAFT_CACHE_DIRcan neverrm -rfuser data. Now shown in the §5.4 tree, documented in §4.7 clean, and mentioned in testing.md'scache dirrow.Content-store sharding clarified
store/sha256/<xx>/<hex…>/reads as "full hash under a 2-char bucket", but the entry directory is actually the remaining 62 digits (hash4e13…→store/sha256/4e/13…/). The §5.4 content-store paragraph now spells this out.testing.md scenario fixes
add mint@main) was order-dependent: on an existing entry, a branch ref resolving to the already-locked commit hits the spec §4.2 "already at" no-op and keeps the tag version, so the old sequence could not reproduce a pseudo-version. It now removes the dep first, with a note explaining the no-op interaction.Verification
go test -race ./internal/clierr/passes — both conformance ratchets (§4.6 exit-code table, REQ coverage including the new REQ-ADD-NOREPOINT).go test -race -run TestAdd_repointWithNameRejected ./cmd/graft/passes.go build ./...passes.🤖 Generated with Claude Code